Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Github Co-pilot to add Docstrings and Type Hints #316

Merged
merged 31 commits into from
Aug 3, 2024
Merged

Conversation

jan-janssen
Copy link
Member

@jan-janssen jan-janssen commented Aug 3, 2024

Summary by CodeRabbit

  • New Features

    • Introduced new configuration settings for Python testing in Visual Studio Code to streamline unit testing.
    • Added functions for optimizing atomic positions and volumes with ASE optimizers.
    • Enhanced the ThermoBulk class with refined method signatures and improved usability in thermodynamic calculations.
  • Documentation

    • Enhanced documentation across various classes and functions, including detailed docstrings and type annotations, improving usability and clarity for developers.
  • Bug Fixes

    • Improved error handling and input validation in key functions to enhance robustness.
    • Updated function return types for clarity in output handling.
  • Refactor

    • Improved code structure and consistency by adding type hints and enhancing method signatures across multiple files.

Copy link

coderabbitai bot commented Aug 3, 2024

Warning

Rate limit exceeded

@jan-janssen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 52 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 8d36e83 and 950ab00.

Walkthrough

The recent updates significantly enhance the codebase by improving documentation and type annotations across various modules. Comprehensive docstrings have been added to functions and classes, clarifying their purposes and usage. Type hints were introduced for better type safety, particularly in testing, scientific calculations, and structure optimization, making the code more maintainable and user-friendly.

Changes

Files Summary
.vscode/settings.json Added unittest configuration for Python testing, defining test directory and patterns.
atomistics/calculators/ase.py Enhanced ASEExecutor with type annotations and detailed docstrings; introduced new optimization functions.
atomistics/calculators/hessian.py Added detailed docstrings and refined return types for functions, improving clarity and error handling.
atomistics/calculators/interface.py Added docstring to get_quantities_from_tasks, enhancing documentation without logic changes.
atomistics/calculators/qe.py Improved documentation and type annotations for QEStaticParser methods.
atomistics/shared/output.py Enhanced Output class with type-hinted return types and detailed docstrings.
atomistics/shared/thermal_expansion.py Added comprehensive docstrings to ThermalExpansionProperties methods and related functions.
atomistics/shared/tqdm_iterator.py Enhanced documentation for get_tqdm_iterator function.
atomistics/workflows/elastic/*.py Improved documentation and type hints in functions related to elastic property calculations.
atomistics/workflows/evcurve/thermo.py Enhanced ThermoBulk class with type hints and detailed docstrings for methods, improving usability.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CodeBase
    participant Tests
    User->>CodeBase: Run Tests
    CodeBase->>Tests: Execute unittest
    Tests->>CodeBase: Return test results
    CodeBase->>User: Show test results
Loading

🐇 In the meadow, where rabbits play,
A codebase brightens the day!
With docstrings added, oh so neat,
Each function dances on tiny feet.
Type hints sparkle, errors take flight,
Hooray for clarity, a coder's delight! ✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 18

Outside diff range, codebase verification and nitpick comments (1)
atomistics/workflows/quasiharmonic.py (1)

Line range hint 749-768: Fix undefined name Thermal.

The name Thermal is not defined in the get_thermal_properties method. Ensure that Thermal is imported or defined appropriately.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2aedb31 and b4f5043.

Files selected for processing (24)
  • .vscode/settings.json (1 hunks)
  • atomistics/calculators/ase.py (8 hunks)
  • atomistics/calculators/hessian.py (7 hunks)
  • atomistics/calculators/interface.py (1 hunks)
  • atomistics/calculators/qe.py (6 hunks)
  • atomistics/shared/output.py (1 hunks)
  • atomistics/shared/thermal_expansion.py (1 hunks)
  • atomistics/shared/tqdm_iterator.py (1 hunks)
  • atomistics/workflows/elastic/elastic_moduli.py (5 hunks)
  • atomistics/workflows/elastic/helper.py (5 hunks)
  • atomistics/workflows/elastic/symmetry.py (5 hunks)
  • atomistics/workflows/elastic/workflow.py (3 hunks)
  • atomistics/workflows/evcurve/debye.py (10 hunks)
  • atomistics/workflows/evcurve/fit.py (17 hunks)
  • atomistics/workflows/evcurve/helper.py (10 hunks)
  • atomistics/workflows/evcurve/thermo.py (21 hunks)
  • atomistics/workflows/evcurve/workflow.py (4 hunks)
  • atomistics/workflows/interface.py (1 hunks)
  • atomistics/workflows/langevin.py (5 hunks)
  • atomistics/workflows/molecular_dynamics.py (1 hunks)
  • atomistics/workflows/phonons/helper.py (16 hunks)
  • atomistics/workflows/phonons/workflow.py (10 hunks)
  • atomistics/workflows/quasiharmonic.py (15 hunks)
  • atomistics/workflows/structure_optimization.py (1 hunks)
Files skipped from review due to trivial changes (7)
  • .vscode/settings.json
  • atomistics/calculators/interface.py
  • atomistics/shared/thermal_expansion.py
  • atomistics/shared/tqdm_iterator.py
  • atomistics/workflows/interface.py
  • atomistics/workflows/molecular_dynamics.py
  • atomistics/workflows/structure_optimization.py
Additional context used
Ruff
atomistics/workflows/elastic/helper.py

147-147: Undefined name List

(F821)


147-147: Undefined name List

(F821)


147-147: Undefined name Optional

(F821)

atomistics/workflows/langevin.py

155-155: Undefined name Dict

(F821)


155-155: Undefined name Dict

(F821)


188-188: Undefined name Dict

(F821)


188-188: Undefined name Dict

(F821)

atomistics/calculators/hessian.py

209-209: Undefined name Union

(F821)

atomistics/workflows/evcurve/helper.py

108-108: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


351-351: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

atomistics/workflows/evcurve/debye.py

354-354: Undefined name Dict

(F821)


354-354: Undefined name List

(F821)

atomistics/workflows/evcurve/thermo.py

19-19: Undefined name Optional

(F821)


20-20: Undefined name Optional

(F821)


21-21: Undefined name Optional

(F821)


22-22: Undefined name Optional

(F821)


23-23: Undefined name Optional

(F821)


24-24: Undefined name Optional

(F821)


444-444: Undefined name plt

(F821)


494-494: Undefined name DebyeModel

(F821)

atomistics/workflows/elastic/elastic_moduli.py

304-304: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


314-314: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


324-324: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


334-334: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


344-344: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


356-356: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


369-369: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


382-382: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


395-395: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


408-408: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


421-421: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


434-434: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


446-446: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


458-458: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)


471-471: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks

(B019)

atomistics/calculators/ase.py

198-198: Do not perform function call OutputStatic.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


212-212: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


267-267: Undefined name Calculator

(F821)


273-273: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)


369-369: Do not perform function call OutputThermalExpansion.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


427-427: Do not perform function call np.array in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


428-428: Do not perform function call OutputMolecularDynamics.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


495-495: Loop control variable i not used within loop body

Rename unused i to _i

(B007)


501-501: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

atomistics/workflows/phonons/helper.py

450-450: Undefined name matplotlib

(F821)


452-452: Undefined name matplotlib

(F821)


533-533: Undefined name matplotlib

(F821)


537-537: Undefined name matplotlib

(F821)

atomistics/workflows/quasiharmonic.py

250-250: Loop control variable t not used within loop body

Rename unused t to _t

(B007)


252-252: Loop control variable w not used within loop body

Rename unused w to _w

(B007)


749-749: Undefined name Thermal

(F821)

Additional comments not posted (106)
atomistics/workflows/elastic/workflow.py (3)

21-30: Docstring addition looks good.

The docstring clearly describes the purpose of each argument and their default values.


44-47: Docstring addition looks good.

The docstring clearly describes the return value of the method.


62-70: Docstring addition looks good.

The docstring clearly describes the parameters and the return value of the method.

atomistics/shared/output.py (2)

11-17: Docstring and type hint addition look good.

The docstring clearly describes the return value of the method, and the type hint improves type clarity.


21-29: Docstring and type hint addition look good.

The docstring clearly describes the parameters and the return value of the method, and the type hint improves type clarity.

atomistics/workflows/evcurve/workflow.py (4)

29-40: Docstring addition looks good.

The docstring clearly describes the purpose of each argument and their default values.


63-66: Docstring addition looks good.

The docstring clearly describes the return value of the method.


82-91: Docstring addition looks good.

The docstring clearly describes the parameters and the return value of the method.


110-123: Docstring addition looks good.

The docstring clearly describes the parameters and the return value of the method.

atomistics/workflows/elastic/helper.py (4)

23-35: Docstring and Type Hints Approved

The added type hints and expanded docstring for generate_structures_helper improve clarity and maintainability.


108-121: Docstring and Type Hints Approved

The added type hints and expanded docstring for analyse_structures_helper improve clarity and maintainability.


187-196: Docstring and Type Hints Approved

The added type hints and expanded docstring for _subjob_name improve clarity and maintainability.


202-214: Docstring and Type Hints Approved

The added type hints and expanded docstring for _fit_elastic_matrix improve clarity and maintainability.

atomistics/workflows/langevin.py (4)

18-28: Docstring and Type Hints Approved

The added type hints and expanded docstring for langevin_delta_v improve clarity and maintainability.


48-57: Docstring and Type Hints Approved

The added type hints and expanded docstring for convert_to_acceleration improve clarity and maintainability.


64-74: Docstring and Type Hints Approved

The added type hints and expanded docstring for get_initial_velocities improve clarity and maintainability.


87-98: Docstring and Type Hints Approved

The added type hints and expanded docstring for get_first_half_step improve clarity and maintainability.

atomistics/calculators/hessian.py (6)

10-22: Docstring Approved

The expanded docstring for check_force_constants improves clarity and maintainability.


53-62: Docstring Approved

The expanded docstring for get_displacement improves clarity and maintainability.


71-82: Docstring and Type Hints Approved

The added type hints and expanded docstring for calc_forces_transformed improve clarity and maintainability.


94-105: Docstring Approved

The expanded docstring for get_forces improves clarity and maintainability.


121-133: Docstring Approved

The expanded docstring for get_energy_pot improves clarity and maintainability.


153-163: Docstring Approved

The expanded docstring for get_stiffness_tensor improves clarity and maintainability.

atomistics/workflows/phonons/workflow.py (11)

46-60: Docstring and type hints added to __init__ method.

The added type hints and detailed docstring improve the clarity and maintainability of the code.


72-77: Docstring added to generate_structures method.

The added docstring accurately describes the method's functionality and return type.


92-99: Docstring added to analyse_structures method.

The added docstring accurately describes the method's functionality, arguments, and return type.


115-137: Docstring and type hints added to get_thermal_properties method.

The added type hints and detailed docstring improve the clarity and maintainability of the code.


154-160: Docstring and return type hint added to get_dynamical_matrix method.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


174-180: Docstring added to dynamical_matrix_at_q method.

The added docstring accurately describes the method's functionality, arguments, and return type.


185-192: Docstring and type hints added to write_phonopy_force_constants method.

The added type hints and detailed docstring improve the clarity and maintainability of the code.


201-206: Docstring added to get_hesse_matrix method.

The added docstring accurately describes the method's functionality and return type.


215-225: Docstring added to get_band_structure method.

The added docstring accurately describes the method's functionality, arguments, and return type.


236-245: Docstring added to plot_band_structure method.

The added docstring accurately describes the method's functionality, arguments, and return type.


265-273: Docstring added to plot_dos method.

The added docstring accurately describes the method's functionality, arguments, and return type.

atomistics/workflows/evcurve/helper.py (10)

14-33: Docstring and error handling added to _strain_axes function.

The added docstring and improved error handling enhance the clarity and robustness of the code.


Line range hint 39-68:
Docstring and type hints added to apply_strain function.

The added type hints and detailed docstring improve the clarity and maintainability of the code.


96-107: Docstring and return type hint added to get_energy_lst function.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


111-121: Docstring and return type hint added to get_volume_lst function.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


128-140: Docstring and return type hint added to fit_ev_curve_internal function.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


152-164: Docstring and return type hint added to fit_ev_curve function.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


178-192: Docstring and error handling added to get_strains function.

The added docstring and improved error handling enhance the clarity and robustness of the code.


214-230: Docstring and error handling added to generate_structures_helper function.

The added docstring and improved error handling enhance the clarity and robustness of the code.


249-264: Docstring added to analyse_structures_helper function.

The added docstring accurately describes the function's functionality, arguments, and return type.


278-284: Docstring and type hint added to EnergyVolumeCurveProperties class constructor.

The added type hint and detailed docstring improve the clarity and maintainability of the code.

atomistics/calculators/qe.py (5)

19-25: Docstring and type hint added to __init__ method.

The added type hint and detailed docstring improve the clarity and maintainability of the code.


29-34: Docstring and return type hint added to forces method.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


38-43: Docstring and return type hint added to energy method.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


47-52: Docstring and return type hint added to stress method.

The added return type hint and detailed docstring improve the clarity and maintainability of the code.


56-61: Docstring and return type hint added to volume method.

The added return type hint and detailed docstring improve the clarity and maintainability of the code

atomistics/workflows/elastic/symmetry.py (4)

24-32: Docstring for find_symmetry_group_number is accurate.

The added docstring correctly describes the function's purpose, parameters, and return value.


88-96: Docstring for get_symmetry_family_from_SGN is accurate.

The added docstring correctly describes the function's purpose, parameters, and return value.


125-133: Docstring for get_LAG_Strain_List is accurate.

The added docstring correctly describes the function's purpose, parameters, and return value.


367-380: Docstring and return type for symmetry_analysis are accurate.

The added docstring correctly describes the function's purpose, parameters, and return value. The return type hint is also accurate.

atomistics/workflows/evcurve/debye.py (12)

22-34: Docstring for __init__ method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose, parameters, and default values.


48-53: Docstring for free_energy method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose and return value.


60-65: Docstring for temperatures method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose and return value.


69-74: Docstring for entropy method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose and return value.


89-94: Docstring for heat_capacity method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose and return value.


110-115: Docstring for volumes method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose and return value.


123-131: Docstring for _debye_kernel method in DebyeThermalProperties is accurate.

The added docstring correctly describes the method's purpose, parameters, and return value.


169-176: Docstring for __init__ method in DebyeModel is accurate.

The added docstring correctly describes the method's purpose, parameters, and default values.


260-268: Docstring for interpolate method in DebyeModel is accurate.

The added docstring correctly describes the method's purpose, parameters, and return value.


275-280: Docstring for debye_temperature method in DebyeModel is accurate.

The added docstring correctly describes the method's purpose and return value.


321-331: Docstring for energy_vib method in DebyeModel is accurate.

The added docstring correctly describes the method's purpose, parameters, and return value.


381-397: Docstring for get_thermal_properties is accurate.

The added docstring correctly describes the function's purpose, parameters, and return value.

atomistics/workflows/evcurve/thermo.py (8)

28-33: Return type hint for copy method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns an instance of ThermoBulk.


54-57: Return type hint for num_atoms method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns an integer.


74-79: Return type hint for _coeff method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns a numpy array.


86-89: Return type hint for temperatures method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns a numpy array.


96-99: Return type hint for _d_temp method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns a float.


106-109: Return type hint for _d_vol method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns a float.


133-136: Return type hint for volumes method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns a numpy array.


160-163: Return type hint for entropy method in ThermoBulk is accurate.

The added return type hint correctly indicates that the method returns a numpy array.

atomistics/workflows/evcurve/fit.py (8)

14-26: Docstring and type hints are well-defined.

The docstring and type hints for the birchmurnaghan_energy function are clear and provide useful information about the function's purpose, parameters, and return value.


36-48: Docstring and type hints are well-defined.

The docstring and type hints for the vinet_energy function are clear and provide useful information about the function's purpose, parameters, and return value.


56-68: Docstring and type hints are well-defined.

The docstring and type hints for the murnaghan function are clear and provide useful information about the function's purpose, parameters, and return value.


99-111: Docstring and type hints are well-defined.

The docstring and type hints for the pouriertarantola function are clear and provide useful information about the function's purpose, parameters, and return value.


153-162: Docstring and type hints are well-defined.

The docstring and type hints for the interpolate_energy function are clear and provide useful information about the function's purpose, parameters, and return value.


476-485: Docstring and type hints are well-defined.

The docstring and type hints for the fit function in the EnergyVolumeFit class are clear and provide useful information about the function's purpose, parameters, and return value.


534-544: Docstring and type hints are well-defined.

The docstring and type hints for the fit_polynomial function in the EnergyVolumeFit class are clear and provide useful information about the function's purpose, parameters, and return value.


667-677: Docstring and type hints are well-defined.

The docstring and type hints for the get_energy_volume_curve_fit function are clear and provide useful information about the function's purpose, parameters, and return value.

atomistics/workflows/quasiharmonic.py (16)

38-47: Docstring and type hints are well-defined.

The docstring and type hints for the get_free_energy_classical function are clear and provide useful information about the function's purpose, parameters, and return value.


70-92: Docstring and type hints are well-defined.

The docstring and type hints for the get_thermal_properties function are clear and provide useful information about the function's purpose, parameters, and return value.


183-199: Docstring and type hints are well-defined.

The docstring and type hints for the _get_thermal_properties_quantum_mechanical function are clear and provide useful information about the function's purpose, parameters, and return value.


225-240: Docstring and type hints are well-defined.

The docstring and type hints for the _get_thermal_properties_classical function are clear and provide useful information about the function's purpose, parameters, and return value.


443-452: Docstring and type hints are well-defined.

The docstring and type hints for the __init__ method in the QuasiHarmonicThermalProperties class are clear and provide useful information about the method's purpose, parameters, and return value.


460-468: Docstring and type hints are well-defined.

The docstring and type hints for the get_property method in the QuasiHarmonicThermalProperties class are clear and provide useful information about the method's purpose, parameters, and return value.


494-499: Docstring and type hints are well-defined.

The docstring and type hints for the temperatures method in the QuasiHarmonicThermalProperties class are clear and provide useful information about the method's purpose and return value.


503-508: Docstring and type hints are well-defined.

The docstring and type hints for the entropy method in the QuasiHarmonicThermalProperties class are clear and provide useful information about the method's purpose and return value.


512-517: Docstring and type hints are well-defined.

The docstring and type hints for the heat_capacity method in the QuasiHarmonicThermalProperties class are clear and provide useful information about the method's purpose and return value.


521-526: Docstring and type hints are well-defined.

The docstring and type hints for the volumes method in the QuasiHarmonicThermalProperties class are clear and provide useful information about the method's purpose and return value.


540-559: Docstring and type hints are well-defined.

The docstring and type hints for the generate_structures_helper function are clear and provide useful information about the function's purpose, parameters, and return value.


603-618: Docstring and type hints are well-defined.

The docstring and type hints for the analyse_structures_helper function are clear and provide useful information about the function's purpose, parameters, and return value.


648-663: Docstring and type hints are well-defined.

The docstring and type hints for the __init__ method in the QuasiHarmonicWorkflow class are clear and provide useful information about the method's purpose, parameters, and return value.


684-689: Docstring and type hints are well-defined.

The docstring and type hints for the generate_structures method in the QuasiHarmonicWorkflow class are clear and provide useful information about the method's purpose and return value.


715-727: Docstring and type hints are well-defined.

The docstring and type hints for the analyse_structures method in the QuasiHarmonicWorkflow class are clear and provide useful information about the method's purpose, parameters, and return value.


485-490: Fix typo in parameter name.

The parameter name thermal_property should be consistent with the rest of the code.

-        return self.get_property(thermal_property="free_energy")
+        return self.get_property("free_energy")

Likely invalid or redundant comment.

atomistics/workflows/elastic/elastic_moduli.py (3)

243-254: LGTM!

The type hint and docstring improve the clarity and usability of the function.


286-292: LGTM!

The type hint and docstring improve the clarity and usability of the constructor.


472-478: LGTM!

The type hint and docstring improve the clarity and usability of the method.

atomistics/calculators/ase.py (4)

27-34: LGTM!

The type hints and docstring improve the clarity and usability of the constructor.


154-166: LGTM!

The type hint and docstring improve the clarity and usability of the function.


217-239: LGTM!

The type hint and docstring improve the clarity and usability of the function.


241-263: LGTM!

The type hint and docstring improve the clarity and usability of the function.

atomistics/workflows/phonons/helper.py (2)

18-51: LGTM!

The type hints and docstring improve the clarity and usability of the constructor.


265-284: LGTM!

The type hints and docstring improve the clarity and usability of the method.

Comment on lines +147 to +161
) -> Tuple[np.ndarray, np.ndarray, List[List[Tuple[float, float]]], Optional[float]]:
"""
Calculate the elastic matrix and other properties.

Args:
output_dict (dict):
output (tuple):
output_dict (dict): The dictionary containing the output data.
Lag_strain_list (list[float]): The list of Lagrangian strains.
epss (float): The list of strains.
v0 (float): The volume of the unit cell.
LC (str): The lattice constant.
fit_order (int, optional): The order of the polynomial fit. Defaults to 2.
zero_strain_job_name (str, optional): The name of the zero strain job. Defaults to "s_e_0".

Returns:

Tuple[np.ndarray, np.ndarray, List[List[Tuple[float, float]]], Optional[float]]: A tuple containing the elastic matrix, A2 coefficients, strain energy data, and ene0 value.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix Undefined Names

The type hints List and Optional are used but not imported. Add the necessary imports from the typing module.

+ from typing import List, Optional
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
) -> Tuple[np.ndarray, np.ndarray, List[List[Tuple[float, float]]], Optional[float]]:
"""
Calculate the elastic matrix and other properties.
Args:
output_dict (dict):
output (tuple):
output_dict (dict): The dictionary containing the output data.
Lag_strain_list (list[float]): The list of Lagrangian strains.
epss (float): The list of strains.
v0 (float): The volume of the unit cell.
LC (str): The lattice constant.
fit_order (int, optional): The order of the polynomial fit. Defaults to 2.
zero_strain_job_name (str, optional): The name of the zero strain job. Defaults to "s_e_0".
Returns:
Tuple[np.ndarray, np.ndarray, List[List[Tuple[float, float]]], Optional[float]]: A tuple containing the elastic matrix, A2 coefficients, strain energy data, and ene0 value.
+ from typing import List, Optional
) -> Tuple[np.ndarray, np.ndarray, List[List[Tuple[float, float]]], Optional[float]]:
"""
Calculate the elastic matrix and other properties.
Args:
output_dict (dict): The dictionary containing the output data.
Lag_strain_list (list[float]): The list of Lagrangian strains.
epss (float): The list of strains.
v0 (float): The volume of the unit cell.
LC (str): The lattice constant.
fit_order (int, optional): The order of the polynomial fit. Defaults to 2.
zero_strain_job_name (str, optional): The name of the zero strain job. Defaults to "s_e_0".
Returns:
Tuple[np.ndarray, np.ndarray, List[List[Tuple[float, float]]], Optional[float]]: A tuple containing the elastic matrix, A2 coefficients, strain energy data, and ene0 value.
Tools
Ruff

147-147: Undefined name List

(F821)


147-147: Undefined name List

(F821)


147-147: Undefined name Optional

(F821)

Comment on lines +188 to +197
def analyse_structures(self, output_dict: Dict[str, Dict[int, Atoms]]):
"""
Analyzes the structures generated in the Langevin dynamics simulation.

Args:
output_dict (dict): A dictionary containing the output structures.

Returns:
tuple: A tuple containing the potential energy and kinetic energy.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix Undefined Names

The type hint Dict is used but not imported. Add the necessary import from the typing module.

+ from typing import Dict
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def analyse_structures(self, output_dict: Dict[str, Dict[int, Atoms]]):
"""
Analyzes the structures generated in the Langevin dynamics simulation.
Args:
output_dict (dict): A dictionary containing the output structures.
Returns:
tuple: A tuple containing the potential energy and kinetic energy.
"""
+ from typing import Dict
def analyse_structures(self, output_dict: Dict[str, Dict[int, Atoms]]):
"""
Analyzes the structures generated in the Langevin dynamics simulation.
Args:
output_dict (dict): A dictionary containing the output structures.
Returns:
tuple: A tuple containing the potential energy and kinetic energy.
"""
Tools
Ruff

188-188: Undefined name Dict

(F821)


188-188: Undefined name Dict

(F821)

Comment on lines +155 to +160
def generate_structures(self) -> Dict[str, Dict[int, Atoms]]:
"""
Generates the structures for the Langevin dynamics simulation.

Returns:
(dict)
dict: A dictionary containing the generated structures.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix Undefined Names

The type hint Dict is used but not imported. Add the necessary import from the typing module.

+ from typing import Dict
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def generate_structures(self) -> Dict[str, Dict[int, Atoms]]:
"""
Generates the structures for the Langevin dynamics simulation.
Returns:
(dict)
dict: A dictionary containing the generated structures.
+ from typing import Dict
def generate_structures(self) -> Dict[str, Dict[int, Atoms]]:
"""
Generates the structures for the Langevin dynamics simulation.
Returns:
dict: A dictionary containing the generated structures.
Tools
Ruff

155-155: Undefined name Dict

(F821)


155-155: Undefined name Dict

(F821)

Comment on lines +355 to +364
"""
Convert the EnergyVolumeCurveProperties object to a dictionary.

Args:
output_keys (tuple, optional): The keys to include in the output dictionary.
Defaults to OutputEnergyVolumeCurve.keys().

Returns:
dict: The converted dictionary.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring added to to_dict method.

The added docstring accurately describes the method's functionality, arguments, and return type.

Remove unnecessary .keys() call.

The .keys() call is redundant and can be removed as suggested by the static analysis tool.

-            **{k: getattr(self, k) for k in OutputEnergyVolumeCurve.keys()}
+            **{k: getattr(self, k) for k in OutputEnergyVolumeCurve}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
Convert the EnergyVolumeCurveProperties object to a dictionary.
Args:
output_keys (tuple, optional): The keys to include in the output dictionary.
Defaults to OutputEnergyVolumeCurve.keys().
Returns:
dict: The converted dictionary.
"""
"""
Convert the EnergyVolumeCurveProperties object to a dictionary.
Args:
output_keys (tuple, optional): The keys to include in the output dictionary.
Defaults to OutputEnergyVolumeCurve.keys().
Returns:
dict: The converted dictionary.
"""

Comment on lines +209 to +226
) -> dict[str, Union[float, np.ndarray]]:
"""
Evaluate the specified tasks using the Hessian calculator.

Args:
structure (Atoms): The current structure.
tasks (dict[str, dict]): The tasks to evaluate.
structure_equilibrium (Atoms): The equilibrium structure.
force_constants (np.ndarray): The force constants array.
bulk_modulus (float, optional): The bulk modulus. Defaults to 0.0.
shear_modulus (float, optional): The shear modulus. Defaults to 0.0.

Returns:
dict[str, Union[float, np.ndarray]]: The results of the evaluated tasks.

Raises:
ValueError: If the specified tasks are not supported.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix Undefined Names

The type hint Union is used but not imported. Add the necessary import from the typing module.

+ from typing import Union
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
) -> dict[str, Union[float, np.ndarray]]:
"""
Evaluate the specified tasks using the Hessian calculator.
Args:
structure (Atoms): The current structure.
tasks (dict[str, dict]): The tasks to evaluate.
structure_equilibrium (Atoms): The equilibrium structure.
force_constants (np.ndarray): The force constants array.
bulk_modulus (float, optional): The bulk modulus. Defaults to 0.0.
shear_modulus (float, optional): The shear modulus. Defaults to 0.0.
Returns:
dict[str, Union[float, np.ndarray]]: The results of the evaluated tasks.
Raises:
ValueError: If the specified tasks are not supported.
"""
+ from typing import Union
) -> dict[str, Union[float, np.ndarray]]:
"""
Evaluate the specified tasks using the Hessian calculator.
Args:
structure (Atoms): The current structure.
tasks (dict[str, dict]): The tasks to evaluate.
structure_equilibrium (Atoms): The equilibrium structure.
force_constants (np.ndarray): The force constants array.
bulk_modulus (float, optional): The bulk modulus. Defaults to 0.0.
shear_modulus (float, optional): The shear modulus. Defaults to 0.0.
Returns:
dict[str, Union[float, np.ndarray]]: The results of the evaluated tasks.
Raises:
ValueError: If the specified tasks are not supported.
"""
Tools
Ruff

209-209: Undefined name Union

(F821)

Comment on lines 468 to 505
def _calc_molecular_dynamics_with_ase(
dyn,
structure: Atoms,
ase_calculator: ASECalculator,
temperature: float,
run: int,
thermo: int,
output_keys: List[str],
) -> dict:
"""
Perform molecular dynamics simulation using ASE.

Args:
dyn: The ASE dynamics object.
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature (float): The desired temperature in Kelvin.
run (int): The number of MD steps to perform.
thermo (int): The interval at which to print thermodynamic properties.
output_keys (List[str]): The keys of the properties to include in the output dictionary.

Returns:
dict: A dictionary containing the calculated properties at each MD step.
"""
structure.calc = ase_calculator
MaxwellBoltzmannDistribution(atoms=structure, temperature_K=temperature)
cache = {q: [] for q in output_keys}
for i in range(int(run / thermo)):
dyn.run(thermo)
ase_instance = ASEExecutor(
ase_structure=structure, ase_calculator=ase_calculator
)
calc_dict = OutputMolecularDynamics(
**{k: getattr(ase_instance, k) for k in OutputMolecularDynamics.keys()}
).get(output_keys=output_keys)
for k, v in calc_dict.items():
cache[k].append(v)
return {q: np.array(cache[q]) for q in output_keys}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename unused loop control variable i.

The loop control variable i is not used within the loop body. Rename it to _.

-    for i in range(int(run / thermo)):
+    for _ in range(int(run / thermo)):
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _calc_molecular_dynamics_with_ase(
dyn,
structure: Atoms,
ase_calculator: ASECalculator,
temperature: float,
run: int,
thermo: int,
output_keys: List[str],
) -> dict:
"""
Perform molecular dynamics simulation using ASE.
Args:
dyn: The ASE dynamics object.
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature (float): The desired temperature in Kelvin.
run (int): The number of MD steps to perform.
thermo (int): The interval at which to print thermodynamic properties.
output_keys (List[str]): The keys of the properties to include in the output dictionary.
Returns:
dict: A dictionary containing the calculated properties at each MD step.
"""
structure.calc = ase_calculator
MaxwellBoltzmannDistribution(atoms=structure, temperature_K=temperature)
cache = {q: [] for q in output_keys}
for i in range(int(run / thermo)):
dyn.run(thermo)
ase_instance = ASEExecutor(
ase_structure=structure, ase_calculator=ase_calculator
)
calc_dict = OutputMolecularDynamics(
**{k: getattr(ase_instance, k) for k in OutputMolecularDynamics.keys()}
).get(output_keys=output_keys)
for k, v in calc_dict.items():
cache[k].append(v)
return {q: np.array(cache[q]) for q in output_keys}
def _calc_molecular_dynamics_with_ase(
dyn,
structure: Atoms,
ase_calculator: ASECalculator,
temperature: float,
run: int,
thermo: int,
output_keys: List[str],
) -> dict:
"""
Perform molecular dynamics simulation using ASE.
Args:
dyn: The ASE dynamics object.
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature (float): The desired temperature in Kelvin.
run (int): The number of MD steps to perform.
thermo (int): The interval at which to print thermodynamic properties.
output_keys (List[str]): The keys of the properties to include in the output dictionary.
Returns:
dict: A dictionary containing the calculated properties at each MD step.
"""
structure.calc = ase_calculator
MaxwellBoltzmannDistribution(atoms=structure, temperature_K=temperature)
cache = {q: [] for q in output_keys}
for _ in range(int(run / thermo)):
dyn.run(thermo)
ase_instance = ASEExecutor(
ase_structure=structure, ase_calculator=ase_calculator
)
calc_dict = OutputMolecularDynamics(
**{k: getattr(ase_instance, k) for k in OutputMolecularDynamics.keys()}
).get(output_keys=output_keys)
for k, v in calc_dict.items():
cache[k].append(v)
return {q: np.array(cache[q]) for q in output_keys}
Tools
Ruff

495-495: Loop control variable i not used within loop body

Rename unused i to _i

(B007)


501-501: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

Comment on lines +369 to +390
output_keys: List[str] = OutputThermalExpansion.keys(),
) -> dict:
"""
Calculate thermal expansion using molecular dynamics simulation with ASE.

Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature_start (float, optional): The starting temperature in Kelvin. Defaults to 15.0.
temperature_stop (float, optional): The stopping temperature in Kelvin. Defaults to 1500.0.
temperature_step (float, optional): The temperature step size in Kelvin. Defaults to 5.0.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputThermalExpansion.keys().

Returns:
dict: A dictionary containing the calculated thermal expansion properties.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix default argument for output_keys.

Do not perform function call OutputThermalExpansion.keys in argument defaults; instead, perform the call within the function.

-    output_keys: List[str] = OutputThermalExpansion.keys(),
+    output_keys: List[str] = None,
) -> dict:
    if output_keys is None:
        output_keys = OutputThermalExpansion.keys()
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
output_keys: List[str] = OutputThermalExpansion.keys(),
) -> dict:
"""
Calculate thermal expansion using molecular dynamics simulation with ASE.
Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature_start (float, optional): The starting temperature in Kelvin. Defaults to 15.0.
temperature_stop (float, optional): The stopping temperature in Kelvin. Defaults to 1500.0.
temperature_step (float, optional): The temperature step size in Kelvin. Defaults to 5.0.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputThermalExpansion.keys().
Returns:
dict: A dictionary containing the calculated thermal expansion properties.
"""
output_keys: List[str] = None,
) -> dict:
if output_keys is None:
output_keys = OutputThermalExpansion.keys()
"""
Calculate thermal expansion using molecular dynamics simulation with ASE.
Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature_start (float, optional): The starting temperature in Kelvin. Defaults to 15.0.
temperature_stop (float, optional): The stopping temperature in Kelvin. Defaults to 1500.0.
temperature_step (float, optional): The temperature step size in Kelvin. Defaults to 5.0.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputThermalExpansion.keys().
Returns:
dict: A dictionary containing the calculated thermal expansion properties.
"""
Tools
Ruff

369-369: Do not perform function call OutputThermalExpansion.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

Comment on lines +418 to +447
def calc_molecular_dynamics_npt_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
run: int = 100,
thermo: int = 100,
timestep: float = 1 * units.fs,
ttime: float = 100 * units.fs,
pfactor: float = 2e6 * units.GPa * (units.fs**2),
temperature: float = 300.0,
externalstress: np.ndarray = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar,
output_keys: List[str] = OutputMolecularDynamics.keys(),
) -> dict:
"""
Perform NPT molecular dynamics simulation using ASE.

Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
temperature (float, optional): The desired temperature in Kelvin. Defaults to 300.0.
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputMolecularDynamics.keys().

Returns:
dict: A dictionary containing the calculated properties at each MD step.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix default arguments for externalstress and output_keys.

Do not perform function calls in argument defaults; instead, perform the calls within the function.

-    externalstress: np.ndarray = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar,
-    output_keys: List[str] = OutputMolecularDynamics.keys(),
+    externalstress: np.ndarray = None,
+    output_keys: List[str] = None,
) -> dict:
    if externalstress is None:
        externalstress = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar
    if output_keys is None:
        output_keys = OutputMolecularDynamics.keys()
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def calc_molecular_dynamics_npt_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
run: int = 100,
thermo: int = 100,
timestep: float = 1 * units.fs,
ttime: float = 100 * units.fs,
pfactor: float = 2e6 * units.GPa * (units.fs**2),
temperature: float = 300.0,
externalstress: np.ndarray = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar,
output_keys: List[str] = OutputMolecularDynamics.keys(),
) -> dict:
"""
Perform NPT molecular dynamics simulation using ASE.
Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
temperature (float, optional): The desired temperature in Kelvin. Defaults to 300.0.
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputMolecularDynamics.keys().
Returns:
dict: A dictionary containing the calculated properties at each MD step.
"""
def calc_molecular_dynamics_npt_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
run: int = 100,
thermo: int = 100,
timestep: float = 1 * units.fs,
ttime: float = 100 * units.fs,
pfactor: float = 2e6 * units.GPa * (units.fs**2),
temperature: float = 300.0,
externalstress: np.ndarray = None,
output_keys: List[str] = None,
) -> dict:
if externalstress is None:
externalstress = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar
if output_keys is None:
output_keys = OutputMolecularDynamics.keys()
"""
Perform NPT molecular dynamics simulation using ASE.
Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
temperature (float, optional): The desired temperature in Kelvin. Defaults to 300.0.
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputMolecularDynamics.keys().
Returns:
dict: A dictionary containing the calculated properties at each MD step.
"""
Tools
Ruff

427-427: Do not perform function call np.array in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


428-428: Do not perform function call OutputMolecularDynamics.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

Comment on lines 447 to 452
dos_energies: np.ndarray,
dos_total: np.ndarray,
*args,
axis: Optional[matplotlib.axes._subplots.AxesSubplot] = None,
**kwargs,
) -> matplotlib.axes._subplots.AxesSubplot:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix undefined name matplotlib.

The name matplotlib is undefined. Ensure it is imported or defined.

+ import matplotlib
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dos_energies: np.ndarray,
dos_total: np.ndarray,
*args,
axis: Optional[matplotlib.axes._subplots.AxesSubplot] = None,
**kwargs,
) -> matplotlib.axes._subplots.AxesSubplot:
+ import matplotlib
dos_energies: np.ndarray,
dos_total: np.ndarray,
*args,
axis: Optional[matplotlib.axes._subplots.AxesSubplot] = None,
**kwargs,
) -> matplotlib.axes._subplots.AxesSubplot:
Tools
Ruff

450-450: Undefined name matplotlib

(F821)


452-452: Undefined name matplotlib

(F821)

Comment on lines 533 to 537
axis: Optional[matplotlib.axes._subplots.AxesSubplot] = None,
*args,
label: Optional[str] = None,
**kwargs,
):
) -> matplotlib.axes._subplots.AxesSubplot:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix undefined name matplotlib.

The name matplotlib is undefined. Ensure it is imported or defined.

+ import matplotlib
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
axis: Optional[matplotlib.axes._subplots.AxesSubplot] = None,
*args,
label: Optional[str] = None,
**kwargs,
):
) -> matplotlib.axes._subplots.AxesSubplot:
+ import matplotlib
axis: Optional[matplotlib.axes._subplots.AxesSubplot] = None,
*args,
label: Optional[str] = None,
**kwargs,
) -> matplotlib.axes._subplots.AxesSubplot:
Tools
Ruff

533-533: Undefined name matplotlib

(F821)


537-537: Undefined name matplotlib

(F821)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b4f5043 and 5b5c825.

Files selected for processing (2)
  • atomistics/calculators/ase.py (7 hunks)
  • atomistics/workflows/elastic/helper.py (5 hunks)
Additional context used
Ruff
atomistics/workflows/elastic/helper.py

147-147: Undefined name Optional

(F821)

atomistics/calculators/ase.py

152-152: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)


198-198: Do not perform function call OutputStatic.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


212-212: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


273-273: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)


369-369: Do not perform function call OutputThermalExpansion.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


427-427: Do not perform function call np.array in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


428-428: Do not perform function call OutputMolecularDynamics.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


495-495: Loop control variable i not used within loop body

Rename unused i to _i

(B007)


501-501: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

Additional comments not posted (17)
atomistics/workflows/elastic/helper.py (5)

23-35: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


108-121: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


147-161: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.

Tools
Ruff

147-147: Undefined name Optional

(F821)


187-196: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


201-214: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.

atomistics/calculators/ase.py (12)

27-34: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


39-44: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


48-53: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


57-62: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


66-71: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


77-82: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


89-94: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


101-106: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


110-115: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


119-124: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


128-133: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.


137-142: LGTM!

The type annotations and expanded docstrings improve clarity and maintainability.

@@ -1,4 +1,5 @@
from collections import OrderedDict
from typing import Any, Dict, List, Tuple
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix Undefined Name Optional.

The type hint Optional is used but not imported. Add the necessary import from the typing module.

+ from typing import Optional
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from typing import Any, Dict, List, Tuple
from typing import Any, Dict, List, Tuple
from typing import Optional

Comment on lines +369 to +390
output_keys: List[str] = OutputThermalExpansion.keys(),
) -> dict:
"""
Calculate thermal expansion using molecular dynamics simulation with ASE.

Args:
structure (Atoms): The atomic structure to simulate.
ase_calculator (ASECalculator): The ASE calculator to use for energy and force calculations.
temperature_start (float, optional): The starting temperature in Kelvin. Defaults to 15.0.
temperature_stop (float, optional): The stopping temperature in Kelvin. Defaults to 1500.0.
temperature_step (float, optional): The temperature step size in Kelvin. Defaults to 5.0.
run (int, optional): The number of MD steps to perform. Defaults to 100.
thermo (int, optional): The interval at which to print thermodynamic properties. Defaults to 100.
timestep (float, optional): The time step size in fs. Defaults to 1 * units.fs.
ttime (float, optional): The total time for the simulation in fs. Defaults to 100 * units.fs.
pfactor (float, optional): The pressure factor in GPa * fs^2. Defaults to 2e6 * units.GPa * (units.fs**2).
externalstress (np.ndarray, optional): The external stress tensor in bar. Defaults to np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar.
output_keys (List[str], optional): The keys of the properties to include in the output dictionary. Defaults to OutputThermalExpansion.keys().

Returns:
dict: A dictionary containing the calculated thermal expansion properties.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid function calls in argument defaults.

Replace the function calls np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar and OutputThermalExpansion.keys() with None and initialize them within the function.

-    externalstress: np.ndarray = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar,
-    output_keys: List[str] = OutputThermalExpansion.keys(),
+    externalstress: np.ndarray = None,
+    output_keys: List[str] = None,
) -> dict:
    if externalstress is None:
        externalstress = np.array([0.0,

<details>
<summary>Tools</summary>

<details>
<summary>Ruff</summary><blockquote>

369-369: Do not perform function call `OutputThermalExpansion.keys` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +198 to +210
output_keys: list[str] = OutputStatic.keys(),
) -> dict:
"""
Calculate static properties using ASE calculator.

Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
output_keys (list[str], optional): List of output keys. Defaults to OutputStatic.keys().

Returns:
dict: Dictionary containing the calculated static properties.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid function calls in argument defaults.

Replace the function call OutputStatic.keys() with None and initialize it within the function.

-    output_keys: list[str] = OutputStatic.keys(),
+    output_keys: list[str] = None,
) -> dict:
    if output_keys is None:
        output_keys = OutputStatic.keys()
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
output_keys: list[str] = OutputStatic.keys(),
) -> dict:
"""
Calculate static properties using ASE calculator.
Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
output_keys (list[str], optional): List of output keys. Defaults to OutputStatic.keys().
Returns:
dict: Dictionary containing the calculated static properties.
"""
output_keys: list[str] = None,
) -> dict:
if output_keys is None:
output_keys = OutputStatic.keys()
"""
Calculate static properties using ASE calculator.
Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
output_keys (list[str], optional): List of output keys. Defaults to OutputStatic.keys().
Returns:
dict: Dictionary containing the calculated static properties.
"""
Tools
Ruff

198-198: Do not perform function call OutputStatic.keys in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

Comment on lines +149 to +166
tasks: List[str],
ase_calculator: ASECalculator,
ase_optimizer: Optimizer = None,
ase_optimizer_kwargs: dict = {},
) -> dict:
"""
Evaluate tasks using ASE calculator.

Args:
structure (Atoms): The ASE structure object.
tasks (list[TaskName]): List of tasks to evaluate.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer, optional): The ASE optimizer object. Defaults to None.
ase_optimizer_kwargs (dict, optional): Keyword arguments for the ASE optimizer. Defaults to {}.

Returns:
dict: Dictionary containing the results of the evaluated tasks.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid mutable default arguments.

Replace the mutable default argument ase_optimizer_kwargs with None and initialize it within the function.

-    ase_optimizer_kwargs: dict = {},
+    ase_optimizer_kwargs: dict = None,
) -> dict:
    if ase_optimizer_kwargs is None:
        ase_optimizer_kwargs = {}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tasks: List[str],
ase_calculator: ASECalculator,
ase_optimizer: Optimizer = None,
ase_optimizer_kwargs: dict = {},
) -> dict:
"""
Evaluate tasks using ASE calculator.
Args:
structure (Atoms): The ASE structure object.
tasks (list[TaskName]): List of tasks to evaluate.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer, optional): The ASE optimizer object. Defaults to None.
ase_optimizer_kwargs (dict, optional): Keyword arguments for the ASE optimizer. Defaults to {}.
Returns:
dict: Dictionary containing the results of the evaluated tasks.
"""
tasks: List[str],
ase_calculator: ASECalculator,
ase_optimizer: Optimizer = None,
ase_optimizer_kwargs: dict = None,
) -> dict:
if ase_optimizer_kwargs is None:
ase_optimizer_kwargs = {}
"""
Evaluate tasks using ASE calculator.
Args:
structure (Atoms): The ASE structure object.
tasks (list[TaskName]): List of tasks to evaluate.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer, optional): The ASE optimizer object. Defaults to None.
ase_optimizer_kwargs (dict, optional): Keyword arguments for the ASE optimizer. Defaults to {}.
Returns:
dict: Dictionary containing the results of the evaluated tasks.
"""
Tools
Ruff

152-152: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)

Comment on lines +241 to +262
def optimize_positions_and_volume_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
ase_optimizer: Optimizer,
ase_optimizer_kwargs: dict,
) -> Atoms:
"""
Optimize the atomic positions and cell volume of the structure using ASE optimizer.

Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer): The ASE optimizer object.
ase_optimizer_kwargs (dict): Keyword arguments for the ASE optimizer.

Returns:
Atoms: The optimized structure.
"""
structure.set_calculator(ase_calculator)
dyn = ase_optimizer(structure, **ase_optimizer_kwargs)
dyn.run(fmax=0.05, volume=True)
return structure
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid mutable default arguments.

Replace the mutable default argument ase_optimizer_kwargs with None and initialize it within the function.

-    ase_optimizer_kwargs: dict = {},
+    ase_optimizer_kwargs: dict = None,
) -> Atoms:
    if ase_optimizer_kwargs is None:
        ase_optimizer_kwargs = {}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def optimize_positions_and_volume_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
ase_optimizer: Optimizer,
ase_optimizer_kwargs: dict,
) -> Atoms:
"""
Optimize the atomic positions and cell volume of the structure using ASE optimizer.
Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer): The ASE optimizer object.
ase_optimizer_kwargs (dict): Keyword arguments for the ASE optimizer.
Returns:
Atoms: The optimized structure.
"""
structure.set_calculator(ase_calculator)
dyn = ase_optimizer(structure, **ase_optimizer_kwargs)
dyn.run(fmax=0.05, volume=True)
return structure
def optimize_positions_and_volume_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
ase_optimizer: Optimizer,
ase_optimizer_kwargs: dict = None,
) -> Atoms:
"""
Optimize the atomic positions and cell volume of the structure using ASE optimizer.
Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer): The ASE optimizer object.
ase_optimizer_kwargs (dict): Keyword arguments for the ASE optimizer.
Returns:
Atoms: The optimized structure.
"""
if ase_optimizer_kwargs is None:
ase_optimizer_kwargs = {}
structure.set_calculator(ase_calculator)
dyn = ase_optimizer(structure, **ase_optimizer_kwargs)
dyn.run(fmax=0.05, volume=True)
return structure

Comment on lines +217 to +239
def optimize_positions_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
run: int = 100,
thermo: int = 100,
timestep: float = 1 * units.fs,
ttime: float = 100 * units.fs,
pfactor: float = 2e6 * units.GPa * (units.fs**2),
temperature: float = 100.0,
externalstress: np.ndarray = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar,
output_keys=OutputMolecularDynamics.keys(),
) -> dict:
return _calc_molecular_dynamics_with_ase(
dyn=NPT(
atoms=structure,
timestep=timestep,
temperature=None,
externalstress=externalstress,
ttime=ttime,
pfactor=pfactor,
temperature_K=temperature,
mask=None,
trajectory=None,
logfile=None,
loginterval=1,
append_trajectory=False,
),
structure=structure,
ase_calculator=ase_calculator,
temperature=temperature,
run=run,
thermo=thermo,
output_keys=output_keys,
)
ase_optimizer: Optimizer,
ase_optimizer_kwargs: dict,
) -> Atoms:
"""
Optimize the atomic positions of the structure using ASE optimizer.

Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer): The ASE optimizer object.
ase_optimizer_kwargs (dict): Keyword arguments for the ASE optimizer.

Returns:
Atoms: The optimized structure.
"""
structure.set_calculator(ase_calculator)
dyn = ase_optimizer(structure, **ase_optimizer_kwargs)
dyn.run(fmax=0.05)
return structure

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid mutable default arguments.

Replace the mutable default argument ase_optimizer_kwargs with None and initialize it within the function.

-    ase_optimizer_kwargs: dict = {},
+    ase_optimizer_kwargs: dict = None,
) -> Atoms:
    if ase_optimizer_kwargs is None:
        ase_optimizer_kwargs = {}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def optimize_positions_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
run: int = 100,
thermo: int = 100,
timestep: float = 1 * units.fs,
ttime: float = 100 * units.fs,
pfactor: float = 2e6 * units.GPa * (units.fs**2),
temperature: float = 100.0,
externalstress: np.ndarray = np.array([0.0, 0.0, 0.0, 0.0, 0.0, 0.0]) * units.bar,
output_keys=OutputMolecularDynamics.keys(),
) -> dict:
return _calc_molecular_dynamics_with_ase(
dyn=NPT(
atoms=structure,
timestep=timestep,
temperature=None,
externalstress=externalstress,
ttime=ttime,
pfactor=pfactor,
temperature_K=temperature,
mask=None,
trajectory=None,
logfile=None,
loginterval=1,
append_trajectory=False,
),
structure=structure,
ase_calculator=ase_calculator,
temperature=temperature,
run=run,
thermo=thermo,
output_keys=output_keys,
)
ase_optimizer: Optimizer,
ase_optimizer_kwargs: dict,
) -> Atoms:
"""
Optimize the atomic positions of the structure using ASE optimizer.
Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer): The ASE optimizer object.
ase_optimizer_kwargs (dict): Keyword arguments for the ASE optimizer.
Returns:
Atoms: The optimized structure.
"""
structure.set_calculator(ase_calculator)
dyn = ase_optimizer(structure, **ase_optimizer_kwargs)
dyn.run(fmax=0.05)
return structure
def optimize_positions_with_ase(
structure: Atoms,
ase_calculator: ASECalculator,
ase_optimizer: Optimizer,
ase_optimizer_kwargs: dict = None,
) -> Atoms:
"""
Optimize the atomic positions of the structure using ASE optimizer.
Args:
structure (Atoms): The ASE structure object.
ase_calculator (ASECalculator): The ASE calculator object.
ase_optimizer (Optimizer): The ASE optimizer object.
ase_optimizer_kwargs (dict): Keyword arguments for the ASE optimizer.
Returns:
Atoms: The optimized structure.
"""
if ase_optimizer_kwargs is None:
ase_optimizer_kwargs = {}
structure.set_calculator(ase_calculator)
dyn = ase_optimizer(structure, **ase_optimizer_kwargs)
dyn.run(fmax=0.05)
return structure

Comment on lines 270 to 290
timestep: float = 1.0,
temperature: float = 100.0,
friction: float = 0.002,
output_keys=OutputMolecularDynamics.keys(),
):
output_keys: List[str] = ["energy", "forces", "positions"],
) -> dict:
"""
Perform molecular dynamics simulation using the Langevin algorithm with ASE.

Parameters:
- structure (Atoms): The atomic structure to simulate.
- ase_calculator (Calculator): The ASE calculator to use for energy and force calculations.
- run (int): The number of MD steps to perform.
- thermo (int): The interval at which to print thermodynamic properties.
- timestep (float): The time step size in fs.
- temperature (float): The desired temperature in Kelvin.
- friction (float): The friction coefficient for the Langevin thermostat.
- output_keys (List[str]): The keys of the properties to include in the output dictionary.

Returns:
- dict: A dictionary containing the calculated properties at each MD step.
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid mutable default arguments.

Replace the mutable default argument output_keys with None and initialize it within the function.

-    output_keys: List[str] = ["energy", "forces", "positions"],
+    output_keys: List[str] = None,
) -> dict:
    if output_keys is None:
        output_keys = ["energy", "forces", "positions"]
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
timestep: float = 1.0,
temperature: float = 100.0,
friction: float = 0.002,
output_keys=OutputMolecularDynamics.keys(),
):
output_keys: List[str] = ["energy", "forces", "positions"],
) -> dict:
"""
Perform molecular dynamics simulation using the Langevin algorithm with ASE.
Parameters:
- structure (Atoms): The atomic structure to simulate.
- ase_calculator (Calculator): The ASE calculator to use for energy and force calculations.
- run (int): The number of MD steps to perform.
- thermo (int): The interval at which to print thermodynamic properties.
- timestep (float): The time step size in fs.
- temperature (float): The desired temperature in Kelvin.
- friction (float): The friction coefficient for the Langevin thermostat.
- output_keys (List[str]): The keys of the properties to include in the output dictionary.
Returns:
- dict: A dictionary containing the calculated properties at each MD step.
"""
timestep: float = 1.0,
temperature: float = 100.0,
friction: float = 0.002,
output_keys: List[str] = None,
) -> dict:
if output_keys is None:
output_keys = ["energy", "forces", "positions"]
"""
Perform molecular dynamics simulation using the Langevin algorithm with ASE.
Parameters:
- structure (Atoms): The atomic structure to simulate.
- ase_calculator (Calculator): The ASE calculator to use for energy and force calculations.
- run (int): The number of MD steps to perform.
- thermo (int): The interval at which to print thermodynamic properties.
- timestep (float): The time step size in fs.
- temperature (float): The desired temperature in Kelvin.
- friction (float): The friction coefficient for the Langevin thermostat.
- output_keys (List[str]): The keys of the properties to include in the output dictionary.
Returns:
- dict: A dictionary containing the calculated properties at each MD step.
"""
Tools
Ruff

273-273: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5b5c825 and f2cccf7.

Files selected for processing (2)
  • atomistics/calculators/hessian.py (7 hunks)
  • atomistics/workflows/elastic/helper.py (5 hunks)
Files skipped from review as they are similar to previous changes (2)
  • atomistics/calculators/hessian.py
  • atomistics/workflows/elastic/helper.py

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f2cccf7 and 8d36e83.

Files selected for processing (1)
  • atomistics/workflows/evcurve/thermo.py (21 hunks)
Additional context used
Ruff
atomistics/workflows/evcurve/thermo.py

19-19: Undefined name Optional

(F821)


20-20: Undefined name Optional

(F821)


21-21: Undefined name Optional

(F821)


22-22: Undefined name Optional

(F821)


23-23: Undefined name Optional

(F821)


24-24: Undefined name Optional

(F821)

Additional comments not posted (23)
atomistics/workflows/evcurve/thermo.py (23)

9-9: LGTM!

The class definition has been simplified by removing explicit object inheritance. This is a good change as it makes the code cleaner.


28-31: LGTM!

The copy method has been updated with a return type hint and a docstring. This improves readability and maintainability.


44-44: LGTM!

The _reset_energy method has been updated with a docstring. This improves readability and maintainability.


54-57: LGTM!

The num_atoms property has been updated with a return type hint and a docstring. This improves readability and maintainability.


66-69: LGTM!

The num_atoms setter has been updated with a docstring. This improves readability and maintainability.


74-79: LGTM!

The _coeff property has been updated with a return type hint and a docstring. This improves readability and maintainability.


86-89: LGTM!

The temperatures property has been updated with a return type hint and a docstring. This improves readability and maintainability.


96-99: LGTM!

The _d_temp property has been updated with a return type hint and a docstring. This improves readability and maintainability.


106-109: LGTM!

The _d_vol property has been updated with a return type hint and a docstring. This improves readability and maintainability.


116-119: LGTM!

The temperatures setter has been updated with a docstring. This improves readability and maintainability.


133-136: LGTM!

The volumes property has been updated with a return type hint and a docstring. This improves readability and maintainability.


143-146: LGTM!

The volumes setter has been updated with a docstring. This improves readability and maintainability.


160-163: LGTM!

The entropy property has been updated with a return type hint and a docstring. This improves readability and maintainability.


172-175: LGTM!

The pressure property has been updated with a return type hint and a docstring. This improves readability and maintainability.


184-187: LGTM!

The energies property has been updated with a return type hint and a docstring. This improves readability and maintainability.


194-197: LGTM!

The energies setter has been updated with a docstring. This improves readability and maintainability.


218-223: LGTM!

The set_temperatures method has been updated with a docstring. This improves readability and maintainability.


233-238: LGTM!

The set_volumes method has been updated with a docstring. This improves readability and maintainability.


246-249: LGTM!

The meshgrid method has been updated with a return type hint and a docstring. This improves readability and maintainability.


255-261: LGTM!

The get_minimum_energy_path method has been updated with a return type hint and a docstring. This improves readability and maintainability.


281-288: LGTM!

The get_free_energy method has been updated with a return type hint and a docstring. This improves readability and maintainability.


297-304: LGTM!

The interpolate_volume method has been updated with a return type hint and a docstring. This improves readability and maintainability.


493-503: LGTM!

The get_thermo_bulk_model function has been updated with a return type hint and a docstring. This improves readability and maintainability.

Comment on lines +19 to +24
self._volumes: Optional[np.ndarray] = None
self._temperatures: Optional[np.ndarray] = None
self._energies: Optional[np.ndarray] = None
self._entropy: Optional[np.ndarray] = None
self._pressure: Optional[np.ndarray] = None
self._num_atoms: Optional[int] = None
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix undefined name Optional in __init__ method.

The method __init__ uses Optional which is not imported.

+ from typing import Optional
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self._volumes: Optional[np.ndarray] = None
self._temperatures: Optional[np.ndarray] = None
self._energies: Optional[np.ndarray] = None
self._entropy: Optional[np.ndarray] = None
self._pressure: Optional[np.ndarray] = None
self._num_atoms: Optional[int] = None
+ from typing import Optional
self._volumes: Optional[np.ndarray] = None
self._temperatures: Optional[np.ndarray] = None
self._energies: Optional[np.ndarray] = None
self._entropy: Optional[np.ndarray] = None
self._pressure: Optional[np.ndarray] = None
self._num_atoms: Optional[int] = None
Tools
Ruff

19-19: Undefined name Optional

(F821)


20-20: Undefined name Optional

(F821)


21-21: Undefined name Optional

(F821)


22-22: Undefined name Optional

(F821)


23-23: Undefined name Optional

(F821)


24-24: Undefined name Optional

(F821)

@jan-janssen
Copy link
Member Author

======================================================================
ERROR: test_ase_langevin (test_ase_md_emt.TestASEMD.test_ase_langevin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/atomistics/atomistics/tests/test_ase_md_emt.py", line 19, in test_ase_langevin
    result_dict = calc_molecular_dynamics_langevin_with_ase(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/calculators/ase.py", line 291, in calc_molecular_dynamics_langevin_with_ase
    return _calc_molecular_dynamics_with_ase(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/calculators/ase.py", line 542, in _calc_molecular_dynamics_with_ase
    ).get(output_keys=output_keys)
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/shared/output.py", line 30, in get
    return {q: getattr(self, q)() for q in output_keys}
               ^^^^^^^^^^^^^^^^
AttributeError: 'OutputMolecularDynamics' object has no attribute 'energy'

======================================================================
ERROR: test_ase_npt (test_ase_md_emt.TestASEMD.test_ase_npt)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/atomistics/atomistics/tests/test_ase_md_emt.py", line 41, in test_ase_npt
    result_dict = calc_molecular_dynamics_npt_with_ase(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/calculators/ase.py", line 448, in calc_molecular_dynamics_npt_with_ase
    dyn = Langevin(
          ^^^^^^^^^
TypeError: Langevin.__init__() got an unexpected keyword argument 'externalstress'

======================================================================
ERROR: test_calc_phonons (test_quasiharmonic_ase_emt.TestPhonons.test_calc_phonons)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/atomistics/atomistics/tests/test_quasiharmonic_ase_emt.py", line 40, in test_calc_phonons
    thermal_properties_dict = workflow.get_thermal_properties(
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/workflows/quasiharmonic.py", line 774, in get_thermal_properties
    return get_thermal_properties(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/workflows/quasiharmonic.py", line 142, in get_thermal_properties
    np.array([tp_collect_dict[s]["free_energy"][i] for s in strain_lst])
              ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
KeyError: 'free_energy'

----------------------------------------------------------------------

@jan-janssen jan-janssen marked this pull request as draft August 3, 2024 11:14
@jan-janssen
Copy link
Member Author

======================================================================
ERROR: test_ase_npt (test_ase_md_emt.TestASEMD.test_ase_npt)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/atomistics/atomistics/tests/test_ase_md_emt.py", line 41, in test_ase_npt
    result_dict = calc_molecular_dynamics_npt_with_ase(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/calculators/ase.py", line 448, in calc_molecular_dynamics_npt_with_ase
    dyn = Langevin(
          ^^^^^^^^^
TypeError: Langevin.__init__() got an unexpected keyword argument 'externalstress'

======================================================================
ERROR: test_calc_phonons (test_quasiharmonic_ase_emt.TestPhonons.test_calc_phonons)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/atomistics/atomistics/tests/test_quasiharmonic_ase_emt.py", line 40, in test_calc_phonons
    thermal_properties_dict = workflow.get_thermal_properties(
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/workflows/quasiharmonic.py", line 774, in get_thermal_properties
    return get_thermal_properties(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/atomistics/atomistics/atomistics/workflows/quasiharmonic.py", line 142, in get_thermal_properties
    np.array([tp_collect_dict[s]["free_energy"][i] for s in strain_lst])
              ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
KeyError: 'free_energy'

----------------------------------------------------------------------

@jan-janssen jan-janssen marked this pull request as ready for review August 3, 2024 15:02
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jan-janssen jan-janssen merged commit fbe7d50 into main Aug 3, 2024
21 of 22 checks passed
@jan-janssen jan-janssen deleted the copilot branch August 3, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant